Skip to content

Conversation

@yingcong-wu
Copy link
Contributor

We should check the zstd decompress result before doing the msan unpoison. If the res is abnormal, then it would be a huge number, which will cause undesired msan unpoison behavior and will run for a long time.

@llvmbot
Copy link
Member

llvmbot commented Nov 22, 2024

@llvm/pr-subscribers-llvm-support

Author: Wu Yingcong (yingcong-wu)

Changes

We should check the zstd decompress result before doing the msan unpoison. If the res is abnormal, then it would be a huge number, which will cause undesired msan unpoison behavior and will run for a long time.


Full diff: https://github.com/llvm/llvm-project/pull/117276.diff

1 Files Affected:

  • (modified) llvm/lib/Support/Compression.cpp (+4-3)
diff --git a/llvm/lib/Support/Compression.cpp b/llvm/lib/Support/Compression.cpp
index badaf68ab59cd0..3979ca6acaf74e 100644
--- a/llvm/lib/Support/Compression.cpp
+++ b/llvm/lib/Support/Compression.cpp
@@ -206,12 +206,13 @@ Error zstd::decompress(ArrayRef<uint8_t> Input, uint8_t *Output,
   const size_t Res = ::ZSTD_decompress(
       Output, UncompressedSize, (const uint8_t *)Input.data(), Input.size());
   UncompressedSize = Res;
+  if (ZSTD_isError(Res))
+    return make_error<StringError>(ZSTD_getErrorName(Res),
+                                   inconvertibleErrorCode());
   // Tell MemorySanitizer that zstd output buffer is fully initialized.
   // This avoids a false report when running LLVM with uninstrumented ZLib.
   __msan_unpoison(Output, UncompressedSize);
-  return ZSTD_isError(Res) ? make_error<StringError>(ZSTD_getErrorName(Res),
-                                                     inconvertibleErrorCode())
-                           : Error::success();
+  return Error::success();
 }
 
 Error zstd::decompress(ArrayRef<uint8_t> Input,

@yingcong-wu
Copy link
Contributor Author

For the CI failure, I think the test result looks good, the problem seems to be with

python3 /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-mqg5l-1/llvm-project/github-pull-requests/.ci/generate_test_report.py

Does it count as an infrastructure problem?

@yingcong-wu yingcong-wu merged commit 5ed09d5 into llvm:main Nov 25, 2024
8 of 10 checks passed
@yingcong-wu yingcong-wu deleted the yc/1122-zstd-msan-unpoison-before-res-check branch December 4, 2024 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants